fix(ast-engine): correct TAB indentation detection and re-indentation behavior#101
Merged
bashandbone merged 3 commits intofix-indent-tabs-13144396554011743547from Mar 9, 2026
Merged
Conversation
- template.rs:119: use *indent/*is_tab (Copy types) instead of .to_owned()
- indent.rs: fix get_indent_at_offset_with_tab to only set is_tab=true
for pure-tab indentation; mixed indentation falls back to spaces
- indent.rs:331: use get_indent_at_offset_with_tab in test_deindent
for accurate is_tab detection instead of source.contains('\t')
- indent.rs:104-106: update doc comments to reflect tab/mixed support
Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add support for TAB indentation parsing
fix(ast-engine): correct TAB indentation detection and re-indentation behavior
Mar 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR corrects TAB indentation detection and re-indentation behavior in ast-engine’s replacer, ensuring mixed indentation prefixes (spaces + tabs) no longer get treated as pure-tab indentation during reformatting.
Changes:
- Fixes
get_indent_at_offset_with_tabto distinguish between pure-tab vs mixed indentation (space+tab) and only returnis_tab=truefor pure-tab prefixes. - Updates the
test_deindenthelper to deriveis_tabfrom the actual indentation prefix at the extraction point (instead of searching for\tanywhere in the source). - Removes unnecessary
to_owned()calls forCopyvalues in template replacement and refreshes indentation module docs to reflect tab/mixed handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/ast-engine/src/replacer/template.rs | Avoids unnecessary clones by passing Copy values (usize, bool) directly into maybe_get_var. |
| crates/ast-engine/src/replacer/indent.rs | Fixes mixed-indentation detection for tab handling, updates docs, and adjusts a test helper to infer indentation mode from the line prefix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+338
to
+341
| let end = source.chars().count() - trailing_white; | ||
| let extracted = extract_with_deindent(&source, start..end); | ||
| let result_bytes = indent_lines::<String>(0, &extracted, source.contains('\t')); | ||
| let (_, is_tab) = get_indent_at_offset_with_tab::<String>(&source.as_bytes()[..start]); | ||
| let result_bytes = indent_lines::<String>(0, &extracted, is_tab); |
There was a problem hiding this comment.
start/end in test_deindent are derived from .chars().count() and then used as byte offsets (extract_with_deindent(&source, start..end) and &source.as_bytes()[..start]). This is inconsistent for non-ASCII input (multi-byte whitespace or trailing chars) and can panic or produce invalid UTF-8 slices. Consider computing start/end as byte indices (e.g., via char_indices()/len_utf8()), or otherwise deriving the indentation prefix using source.get_range(0..start_byte) where start_byte is guaranteed to be a byte offset.
Contributor
Author
There was a problem hiding this comment.
Fixed in 36da33f. Replaced the .chars().count() arithmetic with str::trim_start / str::trim_end length subtraction so start and end are always byte offsets:
let leading_ws_bytes = source[offset..].len() - source[offset..].trim_start().len();
let start = offset + leading_ws_bytes;
let end = source.trim_end().len();Both values are now guaranteed to be valid UTF-8 byte boundaries, so &source.as_bytes()[..start] and extract_with_deindent(&source, start..end) are safe for non-ASCII input.
Replace .chars().count() with str::trim_start/trim_end length arithmetic so start/end are byte offsets throughout, making the helper correct for non-ASCII / multi-byte UTF-8 input. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
bashandbone
merged commit 8ef87cd
into
fix-indent-tabs-13144396554011743547
2 checks passed
bashandbone
added a commit
that referenced
this pull request
Mar 10, 2026
* feat(ast-engine): support TAB indentation parsing * Added `get_tab` utility function for parsing tab characters from content. * Refactored `get_indent_at_offset` to handle tabs by returning `is_tab` along with indent offset. * Handled the stripping and insertion of mixed-tabs vs space indent characters properly inside `remove_indent` and `indent_lines_impl`. * Plumbed the `is_tab` boolean down through `formatted_slice` and `indent_lines`. * Updated tests in `indent.rs` to exercise proper TAB character extraction and re-indentation rules. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * feat(ast-engine): fix CI failure for rustfmt Fixed formatting issue in `crates/ast-engine/src/replacer/indent.rs` and `crates/ast-engine/src/replacer/template.rs` that was flagged by `cargo fmt --all -- --config-path ./rustfmt.toml --check` in the CI pipeline. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * fix(ast-engine): fix clippy warnings in indent.rs Fixed missing backticks in doc comments and replaced slice allocations (`&[var.clone()]`) with `std::slice::from_ref` inside `crates/ast-engine/src/replacer/indent.rs` to satisfy clippy linters. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * fix: fix clippy errors in flow and language crates Fixed an unused variable warning in `crates/language/src/lib.rs` and collapsed an if-let statement in `crates/flow/src/incremental/analyzer.rs` as mandated by clippy. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * fix: fix clippy errors in flow and language crates Fixed an unused variable warning in `crates/language/src/lib.rs` and collapsed an if-let statement in `crates/flow/src/incremental/analyzer.rs` as mandated by clippy. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * fix(ast-engine): correct TAB indentation detection and re-indentation behavior (#101) * Initial plan * fix(ast-engine): address review comments on TAB indentation support - template.rs:119: use *indent/*is_tab (Copy types) instead of .to_owned() - indent.rs: fix get_indent_at_offset_with_tab to only set is_tab=true for pure-tab indentation; mixed indentation falls back to spaces - indent.rs:331: use get_indent_at_offset_with_tab in test_deindent for accurate is_tab detection instead of source.contains('\t') - indent.rs:104-106: update doc comments to reflect tab/mixed support Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * fix(ast-engine): use byte indices in test_deindent helper Replace .chars().count() with str::trim_start/trim_end length arithmetic so start/end are byte offsets throughout, making the helper correct for non-ASCII / multi-byte UTF-8 input. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * fix: fix clippy errors in flow and language crates Fixed an unused variable warning in `crates/language/src/lib.rs` and collapsed an if-let statement in `crates/flow/src/incremental/analyzer.rs` as mandated by clippy. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> * fix: fix clippy errors in flow and language crates Fixed an unused variable warning in `crates/language/src/lib.rs` and collapsed an if-let statement in `crates/flow/src/incremental/analyzer.rs` as mandated by clippy. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com> --------- Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
template.rs:119– replace.to_owned()with*for Copy typesindent.rs:267-271–is_tab=trueonly for pure-tab indentationindent.rs:331– useget_indent_at_offset_with_tabintest_deindentindent.rs:104-106– update doc comments for tab/mixed supporttest_deindent: computestart/endas byte indices (not char counts) so the helper is correct for non-ASCII input✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.